Fix/guardian proof #432
Fix/guardian proof #432
Conversation
Add the ParameterBaseHash as a static member to the compiled code as well as to the CiphertextElectionContext as a serialized member since we must maintain compatibility with different elections using different parameters for this value.
add parameter hash and sequence ordering to schnorr challenges and propagate the necessary dependencies down the stack
| guardianId, sequenceOrder, | ||
| numberOfGuardians, | ||
| quorum, | ||
| CiphertextElectionContext.ParameterBaseHash, |
There was a problem hiding this comment.
I considered using a default value throughout in the interface but decided against that and to force the application to provide this value because it is runtime dependent (even though it is a compiled constant). This way, the caller is forced to understand that we have a dependency on the values G,Q, and P that are used to make the guardian. Really this probably should be part of the key ceremony and then the guardian can derive agains the key ceremony but i was trying to minimize the number of changes.
bindings/netstandard/ElectionGuard/ElectionGuard.Encryption/Proofs/SchnorrProof.cs
Outdated
Show resolved
Hide resolved
| namespace ElectionGuard | ||
| { | ||
| [StructLayout(LayoutKind.Sequential)] | ||
| public struct ValidationResult |
There was a problem hiding this comment.
this struct is a mirror of the one in the c++ code but it is not yet hooked up so they are not equivalent - they simply have the same structure right now.
|
|
||
| // TODO: update the constant check when hasing is updated to E.G. 2.0 spec. | ||
| // CHECK(context->getParameterHash()->toHex() == | ||
| // "0x2B3B025E50E09C119CBA7E9448ACD1CABC9447EF39BF06327D81C665CDD86296"); |
There was a problem hiding this comment.
in order to get this to validate we need to modify the internal hashing method as part of #433
| /// Raise a ElementModP value to a long exponent | ||
| /// </summary> | ||
| /// <param name="b">base value for the calculation</param> | ||
| /// <param name="e">exponent to raise the base by</param> |
There was a problem hiding this comment.
Since this file was changed, added comments to these methods to remove the linter issues
| var success = PublicKey.IsValidResidue() | ||
| && Commitment.IsInBounds() | ||
| && Response.IsInBounds() | ||
| && validCommitment && validChallenge; | ||
| if (success is false) |
There was a problem hiding this comment.
| if (success is false) | |
| if (!success) |
Note, there shouldn't be a large MSIL difference between the two, if (!foo) is generally more idiomatic and easier to understand.
| var success = PublicKey.IsValidResidue() | ||
| && Commitment.IsInBounds() | ||
| && Response.IsInBounds() | ||
| && validCommitment && validChallenge; |
There was a problem hiding this comment.
I really appreciate this being broken out and used in future stuff
| } | ||
| return success; | ||
| return new ValidationResult() { Success = success, Error = messages }; |
Issue
Link your PR to an issue
Fixes #419
Description
Please describe your pull request.
Adds the hash function parameters to the guardian share challenge per the E.G. 2.0 spec.
The ParameterHash is an input to a number of other hashes and values and so it's both a property of the compiled code (and the election parameters used when using this version of the software to run elections) and also a property of the CiphertextElectionContext itself, which allows for the validation and servicing of elections that use different or earlier parameters. The serialization in the code base will assign the parameterHash when serializing an object in from json but in some cases the value is not used, such as when checking earlier versions of the spec. In the intermediate period it may lead to inconsistent hash validation for elections created against an earlier version. To overcome that limitation it is best to validate against the version of the software used to make the election until E.G. 2.0 is completely implemented.
Testing
Describe the best way to test or validate your PR.